SamUtils PairOrientation is confusingly ambiguous about paired reads needing to map to the same contig #1709
Conversation
…) pertains only to reads on the same contig. - Now getPairOrientation will throw an exception if thrown when the two reads are mapped to different contigs - Added tests to verify that the exception is thrown in the 4 cases specified.
There was a problem hiding this comment.
@yfarjoun There's a typo. This seems like a good improvement otherwise though. Does this require corresponding changes in Picard to avoid hitting the new error case?
An alternate solution would be to add a new state that captures cross chromosome pairs.
| * @return PairOrientation of the given SAMRecord. | ||
| * @throws IllegalArgumentException If the record is not a paired read, or | ||
| * one or both reads are unmapped. | ||
| * one or both reads are unmapped, or is the two records do are not mapped to the |
There was a problem hiding this comment.
typo here: "or is the two records do are not"
|
@lbergelson I don't think that it should be called with reads from different chromosomes. if is it, there's no meaning to RF,FR or TANDEM and so an exception seems (to me) to be the fastest way to find any problems. keeping it in a fourth meaningless state will either
it would be good to see if picard builds (and tests pass) with this version of htsjdk.... want to hlep me with that? |
Description
Please explain the changes you made here.
Explain the motivation for making this change. What existing problem does the pull request solve?
Things to think about before submitting: